Fix uniformtext and enable coloraxis for sunburst and treemap as well as pathbar.textfont #4444

archmoj · 2019-12-26T03:12:26Z

Resolves #4443 by adding coloraxis to sunburst and treemap
and fixes uniformtext scale issues with react (follow up of #4420):

clear previous uniformtext scale before starting each plot type.
fix uniformtext after switching levels when there is no transition for sunburst and treemap
fix tests and add new tests for uniformtext as well as new coloraxis feature.

Also:

fixes pathbar.textfont has no effect in treemap #4451 pathbar.textfont bug in 6a4b451
fixes Problem fitting text inside bar-like traces when textangle is set #4456 textangle bug in 2a7721c

demo treemap
demo sunburst
demo bar

@plotly/plotly_js

- fix clear previous uniformtext scale before each plot - fix uniformtext after switch level when has no transition for sunburst and treemap - fix tests and add new tests

etpinard · 2019-12-27T19:38:48Z

src/traces/bar/plot.js

@@ -91,6 +91,9 @@ function plot(gd, plotinfo, cdModule, traceLayer, opts, makeOnCompleteCallback)
            gap: fullLayout.bargap,
            groupgap: fullLayout.bargroupgap
        };
+
+        // don't clear bar when this is called from waterfall or funnel
+        clearMinTextSize('bar', fullLayout);


Wait. Here you're clearing the min-text-size stash during the plot step. So what's happens when a style-only edit (e.g. Plotly.restyle(gd, 'marker.color', 'red')) gets called?

To me, this logic should probably be somewhere in the calc step.

Restyle works. Considering interactions (e.g. zoom and selection) this should be cleared after calc and at the stat of plot.

Considering interactions (e.g. zoom and selection) this should be cleared after calc and at the stat of plot.

Can you explain a bit more here? To me, sounds like we should not clear minTextSize during zoom interactions.

Or let me rephrase my question: if you do move clearMinTextSize to the calc step, do any of the tests fail?

Yes. In that case the react tests would fail.

Can you share the branch you used to test that?

Summing up a private convo:

I was under the impression that the uniformtext attributes were editType: 'calc', so that's why I thought it would be best to clear the min-text-size during the calc step, but they're not. The uniformtext attributes are editType: 'plot'.

@archmoj says that making the uniformtext attributes editType: 'calc' would have an impact on the way graph with set uniformtext would behave on zoom

Since we'll need to refactor the trace text pipeline at some point (more info in Consistent text mode for bar-like & pie-like traces and feature to control text orientation inside pie/sunburst slices #4420 (comment)), let's keep the clear-min-text logic in the plot step for now.

The most important of this PR are the newly added tests.

etpinard · 2019-12-27T19:39:45Z

src/traces/bar/plot.js

-function transition(selection, opts, makeOnCompleteCallback) {
-    if(hasTransition(opts)) {
+function transition(selection, fullLayout, opts, makeOnCompleteCallback) {
+    if(!fullLayout.uniformtext.mode && hasTransition(opts)) {


Why? I thought you said this transitions looked fine

They did not apply a uniform text size during and after smooth transition.

test/jasmine/tests/sunburst_test.js

test/jasmine/tests/treemap_test.js

src/traces/sunburst/defaults.js

…burst and treemap

…textfont - fixing issue 4451

etpinard · 2019-12-30T15:53:40Z

src/traces/treemap/draw_ancestors.js

@@ -141,7 +141,9 @@ module.exports = function drawAncestors(gd, cd, entry, slices, opts) {
            s.attr('data-notex', 1);
        });

-        var font = Lib.ensureUniformFontSize(gd, helpers.determineTextFont(trace, pt, fullLayout.font, trace.pathdir));
+        var font = Lib.ensureUniformFontSize(gd, helpers.determineTextFont(trace, pt, fullLayout.font, {
+            onPathbar: true


Can you explain why this commit made three existing mocks change (treemap_level-depth, treemap_packings and treemap_textposition) ?

Good question.
And a bad mistake here.
There is no such a thing as pathdir!
It should have been written pathbar.

…r positions - also revise code related to autorotate

archmoj · 2020-01-05T23:08:39Z

After a9c9589 this PR is now ready to go.

test/jasmine/tests/bar_test.js

archmoj · 2020-01-06T17:12:38Z

Here is info related to new/modified baselines:
textangle bug fix (#4456):

(mod) funnel_axis_textangle
(mod) funnel_axis_textangle_start-end
(new) bar-like_textangle45
(new) bar-like_textangle60

treemap trace custom fonts bug fix (#4451)

(new) treemap_fonts_nocolor
(new) treemap_fonts_withcolor
(mod) treemap_packings
(mod) treemap_textposition
(mod) uniformtext_sunburst_treemap

treemap coloraxis with values as well as custom fonts and uniformtext (#4443):

(new) uniformtext_treemap

uniform start/end anchor positions in respect to (#4247):

(mod) niformtext_bar_edgecase2 - text positioned with an identical pad from the top of the bar
(mod) uniformtext_bar_edgecase3 - text positioned with an identical pad from the top of the bar
(new) uniformtext_bar_edgecase4 - text positioned with an identical pad from the bottom of the bar
(new) uniformtext_bar_edgecase5 - text positioned with an identical pad from the bottom of the bar - case of tilted text
(new) uniformtext_bar_edgecase5 - text positioned with an identical pad from the bottom of the bar - case of vertical text on vertical bars
(new) uniformtext_bar_edgecase6 - text positioned with an identical pad from the bottom of the bar - case of horizontal text on horizontal bars
(new) uniformtext_bar_edgecase6 - text positioned with an identical pad from the bottom of the bar - case of tilted text on horizontal bars
(new) uniformtext_bar_axis_textangle_inside - inside text positioned with an identical pad in different axis orientations
(new) uniformtext_bar_axis_textangle_outside - inside text positioned with an identical pad in different axis orientations

modified mock: middle anchors is not used now that the text positions properly by default

(mod) uniformtext_bar-like_10_auto
(mod) uniformtext_bar-like_8_horizontal
(mod) uniformtext_bar-like_8_textangle
(mod) uniformtext_bar-like_8_textangle45

etpinard · 2020-01-06T18:14:02Z

💃 💃

Fixes of uniformtext and enable coloraxis for sunburst and treemap

869b97b

- fix clear previous uniformtext scale before each plot - fix uniformtext after switch level when has no transition for sunburst and treemap - fix tests and add new tests

archmoj added bug something broken feature something new status: reviewable labels Dec 26, 2019

archmoj added this to the v1.52.0 milestone Dec 26, 2019

archmoj added 4 commits December 25, 2019 22:34

no transition with uniformtext for now

b129c5d

only call resizeText not the whole style

c630060

add jasmine tests for sunburst and treemap transition with uniformtext

3737ca9

tweak new test to pass on CI

f6b91e8

etpinard reviewed Dec 27, 2019

View reviewed changes

test/jasmine/tests/sunburst_test.js Show resolved Hide resolved

etpinard reviewed Dec 27, 2019

View reviewed changes

test/jasmine/tests/treemap_test.js Show resolved Hide resolved

etpinard reviewed Dec 27, 2019

View reviewed changes

src/traces/sunburst/defaults.js Outdated Show resolved Hide resolved

archmoj added 3 commits December 27, 2019 17:05

add comment about special logic to consider values colorscales in sun…

ef6a952

…burst and treemap

make new transition tests more robust

2cf7def

treemap pathbar.textfont should have an effect similar to insidetext.…

6a4b451

…textfont - fixing issue 4451

archmoj changed the title ~~Fix uniformtext and enable coloraxis for sunburst and treemap~~ Fix uniformtext and enable coloraxis for sunburst and treemap as well as pathbar.textfont Dec 30, 2019

improve treemap image test with unifromtext and pathbar custom font

16848b3

etpinard reviewed Dec 30, 2019

View reviewed changes

archmoj added 7 commits December 31, 2019 10:39

uniform treemap pathbar text

79ea1cc

textangle bug fix issue 4456

2a7721c

move all uniformtext functions to bar/uniform_text.js

ba1ae6f

declare uniform functions on top of bar plot

599de22

simplify (Inside | Outside) transform logic in bar

0e3e6e2

Merge branch 'master' into more-unifotrm-text

a827bff

keep text pad consistent when uniformtext rescales at start/end ancho…

a9c9589

…r positions - also revise code related to autorotate

etpinard reviewed Jan 6, 2020

View reviewed changes

test/jasmine/tests/bar_test.js Show resolved Hide resolved

etpinard mentioned this pull request Jan 6, 2020

coloraxes with values for sunburst and treemap #4443

Closed

archmoj merged commit 86af82a into master Jan 6, 2020

archmoj deleted the more-unifotrm-text branch January 6, 2020 18:15

jdamiba mentioned this pull request Jan 9, 2020

1.52 Dash QA #4476

Closed

snyk-bot mentioned this pull request Feb 4, 2020

[Snyk] Upgrade plotly.js from 1.45.3 to 1.52.0 beauremus/ftp-viewer#1

Merged

snyk-bot mentioned this pull request Feb 22, 2020

[Snyk] Upgrade plotly.js from 1.35.2 to 1.52.1 raindigi/eShopOnContainersAI#1

Open

snyk-bot mentioned this pull request Mar 18, 2020

[Snyk] Upgrade plotly.js from 1.52.0 to 1.52.2 beauremus/ftp-viewer#19

Open

snyk-bot mentioned this pull request Apr 17, 2020

[Snyk] Upgrade plotly.js from 1.41.3 to 1.52.3 dunzoit/redash#4

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix uniformtext and enable coloraxis for sunburst and treemap as well as pathbar.textfont #4444

Fix uniformtext and enable coloraxis for sunburst and treemap as well as pathbar.textfont #4444

archmoj commented Dec 26, 2019 •

edited

Loading

etpinard Dec 27, 2019

archmoj Dec 29, 2019

etpinard Dec 30, 2019

etpinard Dec 30, 2019

archmoj Jan 5, 2020

etpinard Jan 6, 2020

etpinard Jan 6, 2020

etpinard Dec 27, 2019

archmoj Dec 27, 2019

etpinard Dec 30, 2019

archmoj Dec 30, 2019

archmoj commented Jan 5, 2020

archmoj commented Jan 6, 2020

etpinard commented Jan 6, 2020

Fix uniformtext and enable coloraxis for sunburst and treemap as well as pathbar.textfont #4444

Fix uniformtext and enable coloraxis for sunburst and treemap as well as pathbar.textfont #4444

Conversation

archmoj commented Dec 26, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

archmoj commented Jan 5, 2020

archmoj commented Jan 6, 2020

etpinard commented Jan 6, 2020

archmoj commented Dec 26, 2019 •

edited

Loading